Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #432, Infer OSAL_SYSTEM_OSTYPE from OSAL_SYSTEM_BSPTYPE #436

Merged

Conversation

jphickey
Copy link
Contributor

Describe the contribution

Simplify user configuration by inferring the OSAL_SYSTEM_OSTYPE from the OSAL_SYSTEM_BSPTYPE if the former is not set. This means the user does not have to explicitly set both of these configuration options.

In the event they are both explicitly set (as is done in the current configurations) but refer to an invalid/unexpected combination of OS+BSP, this adds a warning message to the CMake setup (make prep) stage, as this is likely a mis-configuration that deserves attention, but there could be valid reasons for doing this during development.

Fixes #432

Testing performed
Build OSAL for various platform combos (native, x86-linux cross, i686-rtems4.11 cross). Confirm successful build.

Expected behavior changes
No FSW affected, only build script. Also fully backward compatible with old configs.

  • Builds successfully using the inferred OS when only OSAL_SYSTEM_BSPTYPE is set (not OSAL_SYSTEM_OSTYPE).
  • Generates a warning when OSAL_SYSTEM_BSPTYPE and OSAL_SYSTEM_OSTYPE are both set but are mismatched. Confirmed this works by intentionally mismatching in a config.

System(s) tested on

  • Ubuntu 18.04 LTS 64 bit

Additional context
Prerequisite to PR nasa/cFE#634.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

The OSAL_SYSTEM_BSPTYPE specification by itself is enough to
build OSAL, as it implies a specific OS layer.

However, if OSAL_SYSTEM_OSTYPE is explicitly specified, it is used,
but will generate a warning if it is different than the OS layer
implied by the BSP, as there could be reasons for doing this
during development.
@jphickey
Copy link
Contributor Author

This was originally reviewed in the 20200422 CCB, as commit 2f67435 in PR #427 but was rebased so it can be fast-tracked with nasa/cFE#634 to fix a current issue in the master branch.

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB CCB - 20200422 labels Apr 23, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate April 24, 2020 16:34
@astrogeco astrogeco merged commit ea042c9 into nasa:integration-candidate Apr 24, 2020
@jphickey jphickey deleted the fix-432-infer-ostype-from-bsp branch May 14, 2020 13:38
@skliper skliper added this to the 5.1.0 milestone Jun 1, 2020
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Fix nasa#435: Update UT entry point
Resolve merge conflicts in fsw/cfe-core/unit-test/CMakeLists.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB CCB:FastTrack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify config by inferring OSAL_SYSTEM_OSTYPE from OSAL_SYSTEM_BSPTYPE
3 participants